node:child_process: support defining extra pipes #7958
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
❌ @nektro 3 files with test failures on bun-darwin-aarch64:
|
|
❌ @nektro 4 files with test failures on bun-darwin-x64-baseline:
|
|
❌ @nektro 5 files with test failures on bun-darwin-x64:
|
|
❌ @nektro 3 files with test failures on linux-x64:
|
|
❌ @nektro 3 files with test failures on linux-x64-baseline:
|
Jarred-Sumner
added a commit
to Jarred-Sumner/playwright
that referenced
this pull request
Jan 5, 2024
This adds a `package.json` `"exports"` condition for Bun which always chooses the CommonJS version of the export (for both ESM & CJS). This package uses `__esModule` to tell bundlers that it was ESM converted into CommonJS. Bun supports this both at runtime and when using the bundler, but Node.js does not support this at runtime and that causes a compatibility issue when importing `@playwright/test` (which is our fault). It's unfortunately a compatibility issue whether or not we support `__esModule`, because choosing to not support it breaks libraries which presently only work when using a bundler. Since Bun automatically converts CommonJS <> ESM and `module.exports` with dynamic properties are supported, we can choose to always use the CommonJS entry point for Playwright in Bun. There were other changes necessary to get Playwright to work, which are mostly in oven-sh/bun#7958 Signed-off-by: Jarred Sumner <[email protected]>
ryoppippi
pushed a commit
to ryoppippi/bun
that referenced
this pull request
Feb 1, 2024
* node:child_process: support defining extra pipes * unneeded * lazily load node:fs * use $isJSArray instead of ArrayIsArray * remove std.log call * don't close child fd we don't own * close child fd's in parent * add Subprocess.stdio getter that aligns with ChildProcess.stdio fd's * [autofix.ci] apply automated fixes * use ArrayList instead of BoundedArray for stdio_pipes * fix stream primordials * dont use unreachable for syscalls * this file was testing Bun.spawn not child_process.spawn * skip ipc for now * ensure the socketpair is created non-blocking on non-mac posix * allow creating a node:net.Socket from an fd via node:net.connect * node:stream tidy * node:child_process: use net.Socket for stdio instead of fs streams * try again * fix Socket eager loading --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
adds support in
require("node:child_process").spawnfor specifying extra file descriptors that are pipes to be sent to the child process. also fixes other small discrepancies we had with the node api in this area.